Make Evm a part of the supported virtual machines. #3386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Integrate the VM type to the BytecodeId.
ma2bd
reviewed
Feb 22, 2025
ma2bd
reviewed
Feb 22, 2025
ma2bd
reviewed
Feb 23, 2025
| @@ -47,9 +48,14 @@ pub fn create_dummy_user_application_description( | |||
| compressed_bytes: b"service".to_vec(), | |||
| }); | |||
|
|
|||
| let vm_runtime = VmRuntime::default(); | |||
| @@ -350,6 +351,8 @@ pub struct BytecodeId<Abi = (), Parameters = (), InstantiationArgument = ()> { | |||
| pub contract_blob_hash: CryptoHash, | |||
| /// The hash of the blob containing the service bytecode. | |||
| pub service_blob_hash: CryptoHash, | |||
There was a problem hiding this comment.
Maybe, if so it would have to be in a separate PR I think.
Comment on lines
41
to
+44
| record bytecode-id { | ||
| contract-blob-hash: crypto-hash, | ||
| service-blob-hash: crypto-hash, | ||
| vm-runtime: vm-runtime, |
There was a problem hiding this comment.
There was a problem hiding this comment.
Maybe we could change. But in another PR.
linera-client/src/client_options.rs
Outdated
| @@ -773,6 +774,10 @@ pub enum ClientCommand { | |||
| /// Path to the Wasm file for the application "service" bytecode. | |||
| service: PathBuf, | |||
|
|
|||
| /// The virtual machine runtime to use. | |||
| #[arg(long)] | |||
| vm_runtime: Option<VmRuntime>, | |||
There was a problem hiding this comment.
nit: We could also handle the default here.
There was a problem hiding this comment.
You are correct, thank you.
| @@ -1344,19 +1344,6 @@ pub trait WithWasmDefault { | |||
| } | |||
|
|
|||
| impl WasmRuntime { | |||
| #[cfg(with_wasm_runtime)] | |||
| pub fn default_with_sanitizer() -> Self { | |||
|
@MathieuDutSik LGTM. I believe you need to update the summary. Letting someone else accept for good |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
We extend here the support for EVM. This requires no longer making the virtual machine
a feature of the storage but instead a part of the
BytecodeId.Proposal
The following changes have been done:
VmRuntimehas been introduced that covers bothWasm(Native linera) andEvm.VmRuntimeis added to theBytecodeId. This is a reasonable way to make the VM a part of the type description.VmRuntimein theUserApplicationDescriptionsince this forces thecreate_applicationto have aVmRuntimetype. While innocent, this changes is not adequate since thecreate_applicationis used in smart contract code and we want to avoid this part to seep into the smart contract code.VmRuntime,does not depend on any feature. This is because they have to pass the wit barrier and the wit-types cannot depend on features.VmRuntimehas been added to thelinera-base. This is unavoidable becauseBytecodeIddepends on it.load_contract/load_serviceis simplified into one single code that panics when relevant features have not been installed.--vm-runtimeis added to the function calls likepublish_application. When missing it defaults toVmRuntime::Wasm.default_with_stabilizeris eliminated since it is not used.contract_runtime_apis.rsforApplicationDescriptionis corrected.Test Plan
The CI. No test has been added to test the functionality.
Release Plan
Normal release plan.
Links
None.